feat(codex): add Codex native plugin manifest and fix Claude plugin.json#960
Conversation
|
Analysis Failed
Troubleshooting
Retry: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Codex and Claude plugin manifests, a repository-level MCP server configuration, a marketplace entry, packaging updates to include new artifacts, multiple skill front-matter updates, and a test harness that validates plugin/manifest/MCP/marketplace configuration files. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a native Codex plugin manifest ( Key changes:
Confidence Score: 5/5Safe to merge — all previously flagged P1 issues are resolved and the single remaining finding is a minor test-reporting edge case All three previously flagged P1 concerns (double test execution, .mcp.json diagram placement, marketplace path resolving to a non-existent directory) are addressed in the current commit. The only remaining finding is a P2 style issue in the test helper where module-scope loadJsonObject calls can produce ambiguous CI output on JSON parse failure — this does not affect any production path or normal CI runs. tests/plugin-manifest.test.js — minor: module-scope JSON loading could be moved inside test() wrappers for cleaner failure reporting Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
ROOT["Repo Root (plugin root)"]
ROOT --> CP[".claude-plugin/plugin.json\nClaude Code manifest\nagents[], skills[], commands[]"]
ROOT --> CDP[".codex-plugin/plugin.json\nCodex manifest\nskills=string, mcpServers=string"]
ROOT --> MCP[".mcp.json\nMCP server bundle\n6 servers"]
ROOT --> AGT[".agents/plugins/marketplace.json\nCodex marketplace discovery\nsource.path = ../.."]
ROOT --> SK["skills/ (shared)\n125 skills"]
CDP -->|mcpServers = ./.mcp.json| MCP
CDP -->|skills = ./skills/| SK
CP -->|skills = ./skills/| SK
AGT -->|resolves ../.. to repo root| ROOT
MCP --> GH["github\nnpx @modelcontextprotocol/server-github"]
MCP --> CTX["context7\nnpx @upstash/context7-mcp@2.1.4"]
MCP --> EXA["exa\nhttps://mcp.exa.ai/mcp"]
MCP --> MEM["memory\nnpx @modelcontextprotocol/server-memory"]
MCP --> PW["playwright\nnpx @playwright/mcp@0.0.68"]
MCP --> ST["sequential-thinking\nnpx @modelcontextprotocol/server-sequential-thinking"]
Reviews (6): Last reviewed commit: "fix(codex): tighten manifest docs and te..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
package.json
Outdated
| ".agents/", | ||
| ".agents/plugins/marketplace.json", |
There was a problem hiding this comment.
files[] entry
.agents/ (line 41) already recursively includes everything under the .agents/ directory, so the more specific .agents/plugins/marketplace.json entry on line 42 is redundant. npm's files field treats directory entries as recursive globs covering all their contents.
| ".agents/", | |
| ".agents/plugins/marketplace.json", | |
| ".agents/", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
package.json (1)
112-112: Avoid runningplugin-manifest.test.jstwice innpm test.
node tests/run-all.jsalready picks uptests/plugin-manifest.test.js, so the extra explicit call duplicates execution.♻️ Suggested patch
- "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js && node tests/plugin-manifest.test.js", + "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js",As per coding guidelines, "Applies to tests/**/*.test.js : Run tests using Node.js test runner with the command
node tests/run-all.jsfor all tests..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 112, The test script in package.json currently runs node tests/run-all.js and then explicitly runs node tests/plugin-manifest.test.js, causing plugin-manifest.test.js to execute twice; edit the "test" script to remove the redundant explicit invocation of node tests/plugin-manifest.test.js so that only node tests/run-all.js is responsible for running all tests (update the "test" npm script entry to drop the explicit plugin-manifest.test.js call).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex-plugin/README.md:
- Around line 8-11: Update the README tree diagram to show .mcp.json at the repo
root rather than inside .codex-plugin: change the diagram lines referencing
".codex-plugin/ └── .mcp.json" so that .mcp.json appears alongside the
.codex-plugin directory (e.g., list .codex-plugin/ and then on the next
root-level line show .mcp.json), ensuring the manifest plugin.json remains under
.codex-plugin; update the block in README.md that contains the tree diagram
accordingly.
In @.mcp.json:
- Around line 8-10: Replace floating `@latest` tags in the npx args with fixed,
tested versions to ensure reproducible builds: update the args entry that
currently contains "@upstash/context7-mcp@latest" to
"@upstash/context7-mcp@2.1.4" and likewise wherever "@playwright/mcp@latest" is
used change it to "@playwright/mcp@0.0.68" (look for the "command": "npx" entry
and the args array items to edit the package strings).
In `@tests/plugin-manifest.test.js`:
- Around line 134-143: The test 'codex plugin.json mcpServers points to plugin
root .mcp.json (not inside .codex-plugin/)' should assert an exact value rather
than a permissive check: replace the current fuzzy assertion on
codexPlugin.mcpServers and the include/regex logic with a strict equality check
that codexPlugin.mcpServers === './.mcp.json', then continue to resolve mcpPath
using repoRoot and the value and verify fs.existsSync(mcpPath) as before; update
references in the test to use the exact string comparison on
codexPlugin.mcpServers to enforce the contract.
---
Nitpick comments:
In `@package.json`:
- Line 112: The test script in package.json currently runs node tests/run-all.js
and then explicitly runs node tests/plugin-manifest.test.js, causing
plugin-manifest.test.js to execute twice; edit the "test" script to remove the
redundant explicit invocation of node tests/plugin-manifest.test.js so that only
node tests/run-all.js is responsible for running all tests (update the "test"
npm script entry to drop the explicit plugin-manifest.test.js call).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fd72381-488e-435c-9a00-8388b80e5f84
📒 Files selected for processing (7)
.agents/plugins/marketplace.json.claude-plugin/plugin.json.codex-plugin/README.md.codex-plugin/plugin.json.mcp.jsonpackage.jsontests/plugin-manifest.test.js
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:112">
P3: Remove the extra `node tests/plugin-manifest.test.js` invocation from `npm test` to avoid running the same test file twice and inflating CI runtime.</violation>
</file>
<file name="tests/plugin-manifest.test.js">
<violation number="1" location="tests/plugin-manifest.test.js:59">
P3: The new mcpServers test is too permissive: it only rejects paths containing ".codex-plugin" and checks existence, so non-root paths (e.g. ../shared/.mcp.json or ./configs/mcp.json) still pass. This doesn't enforce the stated requirement that mcpServers points to the plugin-root .mcp.json.</violation>
</file>
<file name=".codex-plugin/README.md">
<violation number="1" location=".codex-plugin/README.md:10">
P2: README documents `.mcp.json` under `.codex-plugin/`, but the file actually lives at repo root, creating misleading plugin layout docs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
package.json
Outdated
| "orchestrate:worker": "bash scripts/orchestrate-codex-worker.sh", | ||
| "orchestrate:tmux": "node scripts/orchestrate-worktrees.js", | ||
| "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js", | ||
| "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js && node tests/plugin-manifest.test.js", |
There was a problem hiding this comment.
P3: Remove the extra node tests/plugin-manifest.test.js invocation from npm test to avoid running the same test file twice and inflating CI runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 112:
<comment>Remove the extra `node tests/plugin-manifest.test.js` invocation from `npm test` to avoid running the same test file twice and inflating CI runtime.</comment>
<file context>
@@ -105,7 +109,7 @@
"orchestrate:worker": "bash scripts/orchestrate-codex-worker.sh",
"orchestrate:tmux": "node scripts/orchestrate-worktrees.js",
- "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js",
+ "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js && node tests/plugin-manifest.test.js",
"coverage": "c8 --all --include=\"scripts/**/*.js\" --check-coverage --lines 80 --functions 80 --branches 80 --statements 80 --reporter=text --reporter=lcov node tests/run-all.js"
},
</file context>
| "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js && node tests/plugin-manifest.test.js", | |
| "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-no-personal-paths.js && node scripts/ci/catalog.js --text && node tests/run-all.js", |
senoldogann
force-pushed
the
feat/codex-plugin-manifest
branch
from
4208533 to
c634adf
Compare
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex-plugin/README.md:
- Around line 27-29: Update the README line that shows "codex plugin install ./"
to explicitly state that this command must be executed from the repository root
so that "./" resolves to the repo root and the tool will find .mcp.json
correctly; mention the file name ".mcp.json" and the command "codex plugin
install ./" (or note "run from repo root") so readers inside the .codex-plugin/
directory aren't misled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24ec7e73-b0c9-4239-b6a8-36df012ef5f1
📒 Files selected for processing (7)
.agents/plugins/marketplace.json.claude-plugin/plugin.json.codex-plugin/README.md.codex-plugin/plugin.json.mcp.jsonpackage.jsontests/plugin-manifest.test.js
✅ Files skipped from review due to trivial changes (5)
- package.json
- .agents/plugins/marketplace.json
- .claude-plugin/plugin.json
- .mcp.json
- .codex-plugin/plugin.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/plugin-manifest.test.js
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c634adfa16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| "playwright": { | ||
| "command": "npx", | ||
| "args": ["-y", "@playwright/mcp@0.0.68", "--extension"] |
There was a problem hiding this comment.
The Playwright server is configured with --extension, which switches Playwright MCP into browser-extension bridge mode and requires the Playwright MCP Bridge to be installed. In the common Codex CLI setup (plain npx @playwright/mcp), this makes the bundled Playwright integration unusable for users who have not installed that extension, so Playwright-based workflows in this plugin fail out of the box.
Useful? React with 👍 / 👎.
senoldogann
force-pushed
the
feat/codex-plugin-manifest
branch
from
c634adf to
1e83350
Compare
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e83350291
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.agents/plugins/marketplace.json
Outdated
| "name": "everything-claude-code", | ||
| "source": { | ||
| "source": "local", | ||
| "path": "./everything-claude-code" |
There was a problem hiding this comment.
In the repo marketplace entry, source.path is set to ./everything-claude-code, but this repository does not contain a nested everything-claude-code/ directory, so Codex will resolve this to a non-existent plugin folder and fail to load/install it from the marketplace flow. I checked this against the new marketplace file and the repo root layout: the plugin manifest lives at this repo root (.codex-plugin/plugin.json), so the path should target that real directory (for example ./ or another existing plugin directory).
Useful? React with 👍 / 👎.
senoldogann
force-pushed
the
feat/codex-plugin-manifest
branch
from
1e83350 to
b75c613
Compare
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b75c613579
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| languageName: node | ||
| linkType: hard | ||
| # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | ||
| # yarn lockfile v1 |
There was a problem hiding this comment.
This change downgrades yarn.lock to v1 format (# yarn lockfile v1), while the repo still pins Yarn 4 in package.json (packageManager: yarn@4.9.2) and runs a Yarn install job in CI (.github/workflows/ci.yml). That mismatch makes Yarn Berry treat the lockfile as outdated, causing repeated lockfile rewrites and breaking reproducible/immutable Yarn installs; the lockfile should remain in Berry (__metadata) format.
Useful? React with 👍 / 👎.
| assert.ok(plugin.source && plugin.source.source, `Plugin "${plugin.name}" missing source.source`); | ||
| assert.ok(plugin.policy && plugin.policy.installation, `Plugin "${plugin.name}" missing policy.installation`); |
There was a problem hiding this comment.
The new marketplace test only checks that plugin.source.source exists, but it never validates plugin.source.path for local entries. That allows a broken local path to pass CI and only fail at install time when the marketplace loader tries to resolve the plugin directory. Add an assertion that local sources include a non-empty source.path and that it resolves to an existing directory.
Useful? React with 👍 / 👎.
senoldogann
force-pushed
the
feat/codex-plugin-manifest
branch
from
b75c613 to
f61e1cf
Compare
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.codex-plugin/README.md (1)
27-29:⚠️ Potential issue | 🟡 MinorClarify that local install must run from repository root.
codex plugin install ./is ambiguous when this README is opened inside.codex-plugin/; add a short note that./should be the repo root so.mcp.jsonresolves correctly.🛠️ Suggested patch
-# Or reference locally during development +# Or reference locally during development (run from repository root so ./.mcp.json is found) codex plugin install ./🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex-plugin/README.md around lines 27 - 29, Update the README note that shows "codex plugin install ./" to explicitly state that the command must be run from the repository root so that "./" resolves to the repo root and the .mcp.json file can be located; add one short sentence after the example clarifying "Run this from the repository root (so ./ points to the repo root and .mcp.json resolves correctly)." Reference the same example line ("codex plugin install ./") when adding the clarification.
🧹 Nitpick comments (1)
tests/plugin-manifest.test.js (1)
58-67: Add path traversal guards for manifest-provided agent paths.
agents[]values come from file content; extension checks are good, but../segments should also be rejected before resolving againstrepoRoot.🛠️ Suggested patch
test('claude plugin.json agents uses explicit file paths (not directories)', () => { for (const agentPath of claudePlugin.agents) { + const normalized = path.posix.normalize(agentPath.replace(/\\/g, '/')); assert.ok( agentPath.endsWith('.md'), `Expected explicit .md file path, got: ${agentPath}`, ); assert.ok( !agentPath.endsWith('/'), `Expected explicit file path, not directory, got: ${agentPath}`, ); + assert.ok( + !normalized.startsWith('../') && !normalized.includes('/../'), + `Agent path must not traverse directories: ${agentPath}`, + ); } }); test('claude plugin.json all agent files exist', () => { for (const agentRelPath of claudePlugin.agents) { - const absolute = path.join(repoRoot, agentRelPath.replace(/^\.\//, '')); + const absolute = path.resolve(repoRoot, agentRelPath); + assert.ok( + absolute.startsWith(path.resolve(repoRoot) + path.sep), + `Agent path resolves outside repo root: ${agentRelPath}`, + ); assert.ok( fs.existsSync(absolute), `Agent file missing: ${agentRelPath}`, ); } });As per coding guidelines: “Never trust external data (API responses, user input, file content)”.
Also applies to: 71-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugin-manifest.test.js` around lines 58 - 67, The test currently only checks file extension and lack of trailing slash for entries in claudePlugin.agents; add path-traversal guards to reject any agentPath that contains ".." path segments or is absolute before it would be resolved against repoRoot (e.g., check for patterns like "../" or leading "/" or platform-equivalent path traversal) so the test asserts these are invalid; update the loop over claudePlugin.agents (and the similar check at 71-73) to assert agentPath does not include path traversal segments or absolute paths in addition to the existing .md and directory checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/plugin-manifest.test.js`:
- Line 12: Update the test file header to use the repository-standard test
runner: replace the current "Run with: node tests/plugin-manifest.test.js"
header instruction in tests/plugin-manifest.test.js with "Run with: node
tests/run-all.js" so maintainers use the centralized runner; ensure any
README/top comment in the file references node tests/run-all.js and not the
file-specific node invocation.
- Line 47: Top-level synchronous JSON.parse/fs.readFileSync calls (e.g.,
creating claudePlugin from claudePluginPath and the similar codexPlugin,
mcpConfig, marketplace variables) can crash the test process before the Jest
harness reports failures; move the file reads and JSON.parse into a test-safe
hook (like beforeAll) or into the individual test blocks and handle parse errors
with assertions so failures are reported by the test runner rather than aborting
the suite; update references to claudePlugin, codexPlugin, mcpConfig and
marketplace accordingly so they are assigned inside the hook/test and any
JSON.parse is wrapped in try/catch or assertion logic to surface errors as test
failures.
---
Duplicate comments:
In @.codex-plugin/README.md:
- Around line 27-29: Update the README note that shows "codex plugin install ./"
to explicitly state that the command must be run from the repository root so
that "./" resolves to the repo root and the .mcp.json file can be located; add
one short sentence after the example clarifying "Run this from the repository
root (so ./ points to the repo root and .mcp.json resolves correctly)."
Reference the same example line ("codex plugin install ./") when adding the
clarification.
---
Nitpick comments:
In `@tests/plugin-manifest.test.js`:
- Around line 58-67: The test currently only checks file extension and lack of
trailing slash for entries in claudePlugin.agents; add path-traversal guards to
reject any agentPath that contains ".." path segments or is absolute before it
would be resolved against repoRoot (e.g., check for patterns like "../" or
leading "/" or platform-equivalent path traversal) so the test asserts these are
invalid; update the loop over claudePlugin.agents (and the similar check at
71-73) to assert agentPath does not include path traversal segments or absolute
paths in addition to the existing .md and directory checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79c1fbc3-2ebc-4a16-b89d-7ba18991c00c
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.agents/plugins/marketplace.json.claude-plugin/plugin.json.codex-plugin/README.md.codex-plugin/plugin.json.mcp.jsonpackage.jsontests/plugin-manifest.test.js
✅ Files skipped from review due to trivial changes (4)
- package.json
- .codex-plugin/plugin.json
- .mcp.json
- .claude-plugin/plugin.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/plugins/marketplace.json
tests/plugin-manifest.test.js
Outdated
| assert.ok(fs.existsSync(claudePluginPath), 'Expected .claude-plugin/plugin.json to exist'); | ||
| }); | ||
|
|
||
| const claudePlugin = JSON.parse(fs.readFileSync(claudePluginPath, 'utf8')); |
There was a problem hiding this comment.
Top-level JSON parsing can abort the suite before summary/reporting.
These JSON.parse(fs.readFileSync(...)) calls execute outside test(...), so a missing/invalid file crashes the process and bypasses the harness failure accounting.
🛠️ Suggested refactor pattern
+function parseJsonOrAssert(filePath, label) {
+ try {
+ return JSON.parse(fs.readFileSync(filePath, 'utf8'));
+ } catch (err) {
+ assert.fail(`${label} is unreadable or invalid JSON: ${err.message}`);
+ }
+}
...
-const claudePlugin = JSON.parse(fs.readFileSync(claudePluginPath, 'utf8'));
+let claudePlugin;
+test('claude plugin.json is valid JSON', () => {
+ claudePlugin = parseJsonOrAssert(claudePluginPath, '.claude-plugin/plugin.json');
+});Apply the same pattern to codexPlugin, mcpConfig, and marketplace.
Also applies to: 108-108, 168-168, 197-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/plugin-manifest.test.js` at line 47, Top-level synchronous
JSON.parse/fs.readFileSync calls (e.g., creating claudePlugin from
claudePluginPath and the similar codexPlugin, mcpConfig, marketplace variables)
can crash the test process before the Jest harness reports failures; move the
file reads and JSON.parse into a test-safe hook (like beforeAll) or into the
individual test blocks and handle parse errors with assertions so failures are
reported by the test runner rather than aborting the suite; update references to
claudePlugin, codexPlugin, mcpConfig and marketplace accordingly so they are
assigned inside the hook/test and any JSON.parse is wrapped in try/catch or
assertion logic to surface errors as test failures.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".agents/plugins/marketplace.json">
<violation number="1" location=".agents/plugins/marketplace.json:11">
P1: Local plugin path was changed to a likely invalid location and may break plugin discovery due to incorrect relative pathing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Add .codex-plugin/plugin.json — Codex-native plugin manifest with skills reference and MCP server config pointer - Add .codex-plugin/.mcp.json — standalone MCP server config bundle (github, context7, exa, memory, playwright, sequential-thinking) - Add .codex-plugin/README.md — installation guide and server reference - Fix .claude-plugin/plugin.json — add missing agents[] (28 explicit file paths per validator rules), skills[], and commands[] arrays; remove hooks field (auto-loaded by Claude Code v2.1+ convention) - Add tests/plugin-manifest.test.js — 16 CI tests enforcing PLUGIN_SCHEMA_NOTES.md rules (no hooks, arrays throughout, explicit agent paths, version required, .mcp.json structural checks) - Update package.json: add .codex-plugin/ to files[], add plugin manifest test to npm test chain Refs: .claude-plugin/PLUGIN_SCHEMA_NOTES.md
Codex CLI resolves plugin paths relative to the home directory (~), not relative to marketplace.json. The previous path "./everything-claude-code" resolved to ~/everything-claude-code (non-existent), causing "plugin/read failed" error in the TUI.
Codex CLI requires YAML frontmatter (---) in SKILL.md files. 6 skills were missing frontmatter entirely; laravel-verification had a bare colon in its description causing an invalid YAML parse error.
affaan-m
force-pushed
the
feat/codex-plugin-manifest
branch
from
0e52e22 to
4257c09
Compare
|
Analysis Failed
Troubleshooting
Retry: |
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".codex-plugin/README.md">
<violation number="1" location=".codex-plugin/README.md:30">
P2: Plain prose inside a bash code fence breaks copy-paste execution of the install snippet.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Or reference locally during development | ||
| codex plugin install ./ | ||
|
|
||
| Run this from the repository root so `./` points to the repo root and `.mcp.json` resolves correctly. |
There was a problem hiding this comment.
P2: Plain prose inside a bash code fence breaks copy-paste execution of the install snippet.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codex-plugin/README.md, line 30:
<comment>Plain prose inside a bash code fence breaks copy-paste execution of the install snippet.</comment>
<file context>
@@ -26,6 +26,8 @@ codex plugin install affaan-m/everything-claude-code
# Or reference locally during development
codex plugin install ./
+
+Run this from the repository root so `./` points to the repo root and `.mcp.json` resolves correctly.
</file context>
</details>
```suggestion
# Run this from the repository root so `./` points to the repo root and `.mcp.json` resolves correctly.
2 participants
Summary
This PR adds native Codex plugin support (per the official Codex plugin docs) and fixes a long-standing issue with the Claude plugin manifest.
Problems fixed
1.
.claude-plugin/plugin.jsonwas incompleteThe manifest was missing
agents,skills, andcommandsarrays, which caused the Claude plugin validator to fail on install. Violating rules documented in.claude-plugin/PLUGIN_SCHEMA_NOTES.md.2. No Codex native plugin manifest existed
Codex supports installable plugin bundles but ECC had no
.codex-plugin/directory.Changes (per official Codex plugin docs)
.claude-plugin/plugin.jsonagents[](28 explicit file paths),skills[],commands[]. Removedhooksfield (auto-loaded by Claude Code v2.1+).codex-plugin/plugin.jsonskillsandmcpServersas strings per official spec,interfaceblock for plugin directory.mcp.json.codex-plugin/).agents/plugins/marketplace.json$REPO_ROOT/.agents/plugins/marketplace.json).codex-plugin/README.mdtests/plugin-manifest.test.jspackage.jsonfiles[], added plugin manifest test tonpm testchainType
Testing
All existing tests continue to pass. 22 new tests added and passing:
Checklist
Summary by cubic
Adds a Codex-native plugin manifest with a root
.mcp.jsonand a repo marketplace entry. Fixes the Claude plugin manifest and missing skill metadata, and tightens docs/tests for spec-compliant installs.New Features
.codex-plugin/plugin.json(withskillsandmcpServersas string paths), root.mcp.json,.agents/plugins/marketplace.json, and updated.codex-plugin/README.mdwith placement/install notes.@modelcontextprotocol/server-github,@upstash/context7-mcp@2.1.4,exa,@modelcontextprotocol/server-memory,@playwright/mcp@0.0.68,@modelcontextprotocol/server-sequential-thinking..mcp.jsononly at root,mcpServersmust be"./.mcp.json", marketplace path resolves to repo root).Bug Fixes
.claude-plugin/plugin.jsonwithagents[](explicit.mdpaths),skills[], andcommands[]; removedhooks(auto-loaded in Claude Code v2.1+)..agents/plugins/marketplace.jsonlocalsource.pathso Codex CLI resolves the repo root.Written for commit 52e9bd5. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests